feat: add Cloudflare Workers AI provider (closes #3411)#3604
feat: add Cloudflare Workers AI provider (closes #3411)#3604praveenkumarpranjal wants to merge 9 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Cloudflare Workers AI provider: registers provider, implements CloudflareProvider (chat, streaming, embeddings, responses, and unsupported stubs), adds tests, CI/Docker secret and egress wiring, schema/UI/docs updates, and llmtests account/key/config entries. ChangesCloudflare Workers AI Provider Integration
Sequence DiagramsequenceDiagram
participant Client
participant CloudflareProvider
participant OpenAIHandler
participant CloudflareAPI
Client->>CloudflareProvider: ListModels / Chat / Embedding / Responses
CloudflareProvider->>OpenAIHandler: delegate to shared /v1/* handlers with BaseURL
OpenAIHandler->>CloudflareAPI: HTTP request to account-scoped Cloudflare endpoint
CloudflareAPI-->>OpenAIHandler: HTTP response
OpenAIHandler-->>CloudflareProvider: normalized response
CloudflareProvider-->>Client: Bifrost response or stream
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
|
@praveenkumarpranjal thanks for the PR. Could you move this PR base to dev - and rebase the chagnes |
6ae9bff to
b7b165d
Compare
Confidence Score: 4/5Safe to merge; all request paths are correct and the only finding is a misleading code comment that does not affect runtime behavior. The double-slash URL issue raised in earlier reviews has been fixed: the base URL is stored without core/internal/llmtests/account.go — the comment block in the Cloudflare case of GetConfigForProvider misstates why the test behaves correctly when CLOUDFLARE_ACCOUNT_ID is unset. Important Files Changed
Reviews (6): Last reviewed commit: "review feedback round 3: trim whitespace..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-pipeline.yml:
- Around line 224-225: The test jobs that inject Cloudflare secrets (jobs named
test-core, test-framework, test-plugins, test-api-integrations,
test-docker-image-amd64, test-docker-image-arm64) also require network access to
api.cloudflare.com:443; update each job's harden-runner configuration to add
"api.cloudflare.com:443" to the allowed-endpoints list so Cloudflare provider
tests can reach the API even when egress-policy: block is enabled (apply the
same change where similar blocks exist around the other occurrences referenced
in the comment).
In @.github/workflows/scripts/test-docker-image.sh:
- Around line 154-157: The cloudflare config's base_url currently contains a
literal $CLOUDFLARE_ACCOUNT_ID because the surrounding heredoc is single-quoted;
update the heredoc quoting so shell variables are expanded and use an explicit
variable reference (e.g., ${CLOUDFLARE_ACCOUNT_ID}) in the "base_url" value
inside the "cloudflare" object so the account ID is interpolated at runtime;
ensure you only change the heredoc quoting (to allow expansion) and the base_url
string, leaving other keys (keys, network_config) untouched.
In `@core/providers/cloudflare/cloudflare.go`:
- Line 128: Update the ChatCompletionStream handler to build its request URL
using providerUtils.GetPathFromContext(ctx, "/v1/chat/completions") instead of
directly concatenating provider.networkConfig.BaseURL+"/v1/chat/completions";
locate the ChatCompletionStream function and replace the hardcoded concatenation
with provider.networkConfig.BaseURL+providerUtils.GetPathFromContext(ctx,
"/v1/chat/completions") so it matches how ListModels, ChatCompletion, and
Embedding construct their URLs and respects context-based path overrides.
In `@docs/providers/supported-providers/cloudflare.mdx`:
- Around line 37-57: Update the Cloudflare provider MDX page to include the
required Mintlify tabs "Web UI", "API", and "config.json"; in the Web UI tab
copy the existing prose about Base URL, Max Connections, Idle Timeout and the
note about NewCloudflareProvider returning an error when network_config.base_url
is empty, in the API tab show the Authorization header format ("Authorization:
Bearer <api_token>") and required token scope, and in the config.json tab
provide a concrete JSON example that matches the transports/config.schema.json
(include fields for network_config.base_url, network_config.max_connections,
network_config.idle_timeout_in_seconds/stream_idle_timeout_in_seconds as used by
NewCloudflareProvider); validate the JSON example against
transports/config.schema.json before committing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b16d3a91-34f2-444a-adcd-2d3d44ca5cd4
📒 Files selected for processing (15)
.github/workflows/pr-tests.yml.github/workflows/release-pipeline.yml.github/workflows/scripts/test-docker-image.shcore/bifrost.gocore/providers/cloudflare/cachedcontents.gocore/providers/cloudflare/cloudflare.gocore/providers/cloudflare/cloudflare_test.gocore/schemas/bifrost.godocs/docs.jsondocs/openapi/openapi.jsondocs/providers/supported-providers/cloudflare.mdxtransports/config.schema.jsonui/lib/constants/config.tsui/lib/constants/icons.tsxui/lib/constants/logs.ts
| ## Configuration | ||
|
|
||
| **HTTP Settings:** | ||
|
|
||
| - **Base URL**: required, no default. Set it to `https://api.cloudflare.com/client/v4/accounts/<account_id>/ai/v1`. The account id is the same one shown on the Cloudflare dashboard. | ||
| - **Max Connections**: 5000 per host (matches the rest of the OpenAI-compat providers) | ||
| - **Idle Timeout**: 30 seconds for idle connections; streaming uses the per-stream idle timeout from `network_config.stream_idle_timeout_in_seconds`. | ||
|
|
||
| `NewCloudflareProvider` returns an error if `network_config.base_url` is empty, since there is no global default that omits the account id. | ||
|
|
||
| ## Authentication | ||
|
|
||
| Workers AI accepts a Cloudflare API token as a bearer credential: | ||
|
|
||
| ``` | ||
| Authorization: Bearer <api_token> | ||
| ``` | ||
|
|
||
| The token must have permission to call Workers AI on the target account. Issue tokens from **Cloudflare dashboard → My Profile → API Tokens** and scope them to `Account → Workers AI → Read`. | ||
|
|
||
| --- |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add required Mintlify tabs (Web UI / API / config.json) to this provider page.
This page currently documents config/auth in prose only, but the repo standard requires provider MDX pages to include explicit Web UI / API / config.json tabs (with config.json examples aligned to schema).
As per coding guidelines: "docs/**/*.mdx: Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/providers/supported-providers/cloudflare.mdx` around lines 37 - 57,
Update the Cloudflare provider MDX page to include the required Mintlify tabs
"Web UI", "API", and "config.json"; in the Web UI tab copy the existing prose
about Base URL, Max Connections, Idle Timeout and the note about
NewCloudflareProvider returning an error when network_config.base_url is empty,
in the API tab show the Authorization header format ("Authorization: Bearer
<api_token>") and required token scope, and in the config.json tab provide a
concrete JSON example that matches the transports/config.schema.json (include
fields for network_config.base_url, network_config.max_connections,
network_config.idle_timeout_in_seconds/stream_idle_timeout_in_seconds as used by
NewCloudflareProvider); validate the JSON example against
transports/config.schema.json before committing.
|
Done — retargeted the PR base to cc @akshaydeo |
…verride, harden-runner allowlist Three review fixes from Greptile and CodeRabbit on maximhq#3604: 1. .github/workflows/scripts/test-docker-image.sh — the heredoc that writes config.json is single-quoted (correctly, since `env.XXX` strings are resolved by Bifrost itself), so $CLOUDFLARE_ACCOUNT_ID in the cloudflare base_url was being written literally and the integration test would hit an invalid URL when the secret is set. Substitute it after the heredoc with sed using a non-/ delimiter so the URL slashes don't need escaping. 2. core/providers/cloudflare/cloudflare.go — ChatCompletionStream now builds its URL with providerUtils.GetPathFromContext, matching ChatCompletion / Embedding / ListModels and respecting any context-set path override. 3. .github/workflows/release-pipeline.yml — added api.cloudflare.com:443 to all 4 step-security/harden-runner allowlists that already include api.cerebras.ai:443, so the Cloudflare integration tests can reach the upstream API under the egress-policy: block jobs.
|
Thanks for the reviews. Pushed
I deliberately skipped CodeRabbit's MDX restructure suggestion (Web UI / API / config.json tabs). That pattern is used in this repo for providers with non-trivial auth modes — Azure, Bedrock, Vertex — and skipped for the simple OpenAI-compat providers Cloudflare sits next to (Cerebras, Groq, Mistral, Ollama, etc). Adopting it just for Cloudflare would be inconsistent with the cohort. Happy to add it if the maintainers prefer the broader convention.
|
94db2bd to
5585b3e
Compare
…verride, harden-runner allowlist Three review fixes from Greptile and CodeRabbit on maximhq#3604: 1. .github/workflows/scripts/test-docker-image.sh — the heredoc that writes config.json is single-quoted (correctly, since `env.XXX` strings are resolved by Bifrost itself), so $CLOUDFLARE_ACCOUNT_ID in the cloudflare base_url was being written literally and the integration test would hit an invalid URL when the secret is set. Substitute it after the heredoc with sed using a non-/ delimiter so the URL slashes don't need escaping. 2. core/providers/cloudflare/cloudflare.go — ChatCompletionStream now builds its URL with providerUtils.GetPathFromContext, matching ChatCompletion / Embedding / ListModels and respecting any context-set path override. 3. .github/workflows/release-pipeline.yml — added api.cloudflare.com:443 to all 4 step-security/harden-runner allowlists that already include api.cerebras.ai:443, so the Cloudflare integration tests can reach the upstream API under the egress-policy: block jobs.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/schemas/bifrost.go (1)
51-82:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep provider allowlists consistent for Cloudflare custom-provider configs.
Cloudflare is added to
ModelProvider(Line 51) andStandardProviders(Line 81), and this stack also addscloudflaretocustom_provider_configbase-provider schema enums. ButSupportedBaseProvidersstill omits Cloudflare, which can reject schema-valid custom-provider configs at runtime validation.🔧 Proposed fix
var SupportedBaseProviders = []ModelProvider{ Anthropic, Bedrock, + Cloudflare, Cohere, Gemini, OpenAI, HuggingFace, Replicate, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/schemas/bifrost.go` around lines 51 - 82, SupportedBaseProviders omits Cloudflare while ModelProvider and StandardProviders include it, causing valid cloudflare-backed custom-provider configs to be rejected; update the SupportedBaseProviders slice to include Cloudflare (the ModelProvider value "Cloudflare") so the base-provider allowlist matches StandardProviders and the custom_provider_config enum, ensuring runtime schema validation accepts Cloudflare-based custom providers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/cloudflare/cloudflare.go`:
- Around line 67-73: The provider is shallow-copying config.NetworkConfig into
CloudflareProvider which leaves NetworkConfig.ExtraHeaders shared and can cause
races; update the CloudflareProvider construction to deep-copy the ExtraHeaders
map from config.NetworkConfig (e.g., create a new map, copy entries or use
maps.Copy) and assign that copy to the provider.networkConfig.ExtraHeaders so
the provider owns its own map instance while keeping the rest of
config.NetworkConfig the same.
- Around line 118-145: The streaming call in ChatCompletionStream currently
passes the hardcoded schemas.Cloudflare to
openai.HandleOpenAIChatCompletionStreaming; change that argument to
provider.GetProviderKey() so the provider alias is used consistently (like in
ChatCompletion, ListModels, and Embedding), updating the call in the
ChatCompletionStream function to replace schemas.Cloudflare with
provider.GetProviderKey() so logs/errors and ExtraFields.Provider reflect custom
aliases.
---
Outside diff comments:
In `@core/schemas/bifrost.go`:
- Around line 51-82: SupportedBaseProviders omits Cloudflare while ModelProvider
and StandardProviders include it, causing valid cloudflare-backed
custom-provider configs to be rejected; update the SupportedBaseProviders slice
to include Cloudflare (the ModelProvider value "Cloudflare") so the
base-provider allowlist matches StandardProviders and the custom_provider_config
enum, ensuring runtime schema validation accepts Cloudflare-based custom
providers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e0573df0-7495-4dd6-9e3f-0c026cb40b68
📒 Files selected for processing (12)
.github/workflows/pr-tests.yml.github/workflows/release-pipeline.yml.github/workflows/scripts/test-docker-image.shcore/bifrost.gocore/internal/llmtests/account.gocore/providers/cloudflare/cachedcontents.gocore/providers/cloudflare/cloudflare.gocore/providers/cloudflare/cloudflare_test.gocore/schemas/bifrost.godocs/docs.jsondocs/openapi/openapi.jsondocs/providers/supported-providers/cloudflare.mdx
💤 Files with no reviewable changes (1)
- docs/openapi/openapi.json
✅ Files skipped from review due to trivial changes (1)
- docs/providers/supported-providers/cloudflare.mdx
| config.NetworkConfig.BaseURL = strings.TrimRight(config.NetworkConfig.BaseURL, "/") | ||
|
|
||
| return &CloudflareProvider{ | ||
| logger: logger, | ||
| client: client, | ||
| streamingClient: streamingClient, | ||
| networkConfig: config.NetworkConfig, |
There was a problem hiding this comment.
Deep-copy NetworkConfig.ExtraHeaders to prevent data races.
Line 73 shallow-copies config.NetworkConfig, leaving the ExtraHeaders map shared between the provider and the original config. Concurrent request goroutines reading the map can race if the config is mutated or if other providers share the same config reference.
🔒 Proposed fix to deep-clone the ExtraHeaders map
+import (
+ "maps"
+ // ... existing imports
+)
config.NetworkConfig.BaseURL = strings.TrimRight(config.NetworkConfig.BaseURL, "/")
+ networkConfig := config.NetworkConfig
+ if networkConfig.ExtraHeaders != nil {
+ networkConfig.ExtraHeaders = maps.Clone(networkConfig.ExtraHeaders)
+ }
return &CloudflareProvider{
logger: logger,
client: client,
streamingClient: streamingClient,
- networkConfig: config.NetworkConfig,
+ networkConfig: networkConfig,
sendBackRawRequest: config.SendBackRawRequest,
sendBackRawResponse: config.SendBackRawResponse,
}, nilAs per coding guidelines: "Use deep-copy (maps.Copy) for NetworkConfig.ExtraHeaders and any map fields in config structs to prevent concurrent request data races."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| config.NetworkConfig.BaseURL = strings.TrimRight(config.NetworkConfig.BaseURL, "/") | |
| return &CloudflareProvider{ | |
| logger: logger, | |
| client: client, | |
| streamingClient: streamingClient, | |
| networkConfig: config.NetworkConfig, | |
| config.NetworkConfig.BaseURL = strings.TrimRight(config.NetworkConfig.BaseURL, "/") | |
| networkConfig := config.NetworkConfig | |
| if networkConfig.ExtraHeaders != nil { | |
| networkConfig.ExtraHeaders = maps.Clone(networkConfig.ExtraHeaders) | |
| } | |
| return &CloudflareProvider{ | |
| logger: logger, | |
| client: client, | |
| streamingClient: streamingClient, | |
| networkConfig: networkConfig, | |
| sendBackRawRequest: config.SendBackRawRequest, | |
| sendBackRawResponse: config.SendBackRawResponse, | |
| }, nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/providers/cloudflare/cloudflare.go` around lines 67 - 73, The provider
is shallow-copying config.NetworkConfig into CloudflareProvider which leaves
NetworkConfig.ExtraHeaders shared and can cause races; update the
CloudflareProvider construction to deep-copy the ExtraHeaders map from
config.NetworkConfig (e.g., create a new map, copy entries or use maps.Copy) and
assign that copy to the provider.networkConfig.ExtraHeaders so the provider owns
its own map instance while keeping the rest of config.NetworkConfig the same.
| // ChatCompletionStream performs a streaming chat completion request to | ||
| // Cloudflare Workers AI using the OpenAI-compatible SSE format. | ||
| func (provider *CloudflareProvider) ChatCompletionStream(ctx *schemas.BifrostContext, postHookRunner schemas.PostHookRunner, postHookSpanFinalizer func(context.Context), key schemas.Key, request *schemas.BifrostChatRequest) (chan *schemas.BifrostStreamChunk, *schemas.BifrostError) { | ||
| var authHeader map[string]string | ||
| if key.Value.GetValue() != "" { | ||
| authHeader = map[string]string{"Authorization": "Bearer " + key.Value.GetValue()} | ||
| } | ||
| return openai.HandleOpenAIChatCompletionStreaming( | ||
| ctx, | ||
| provider.streamingClient, | ||
| provider.networkConfig.BaseURL+providerUtils.GetPathFromContext(ctx, "/v1/chat/completions"), | ||
| request, | ||
| authHeader, | ||
| provider.networkConfig.ExtraHeaders, | ||
| provider.networkConfig.StreamIdleTimeoutInSeconds, | ||
| providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest), | ||
| providerUtils.ShouldSendBackRawResponse(ctx, provider.sendBackRawResponse), | ||
| schemas.Cloudflare, | ||
| postHookRunner, | ||
| nil, | ||
| nil, | ||
| nil, | ||
| nil, | ||
| nil, | ||
| provider.logger, | ||
| postHookSpanFinalizer, | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use provider.GetProviderKey() for consistency.
Line 135 hardcodes schemas.Cloudflare instead of calling provider.GetProviderKey(). This is inconsistent with ChatCompletion (line 111), ListModels (line 94), and Embedding (line 157), all of which use provider.GetProviderKey(). If a custom provider alias is configured, streaming error messages and logs will show the wrong provider name.
♻️ Proposed fix
return openai.HandleOpenAIChatCompletionStreaming(
ctx,
provider.streamingClient,
provider.networkConfig.BaseURL+providerUtils.GetPathFromContext(ctx, "/v1/chat/completions"),
request,
authHeader,
provider.networkConfig.ExtraHeaders,
provider.networkConfig.StreamIdleTimeoutInSeconds,
providerUtils.ShouldSendBackRawRequest(ctx, provider.sendBackRawRequest),
providerUtils.ShouldSendBackRawResponse(ctx, provider.sendBackRawResponse),
- schemas.Cloudflare,
+ provider.GetProviderKey(),
postHookRunner,
nil,
nil,
nil,
nil,
nil,
provider.logger,
postHookSpanFinalizer,
)Based on learnings: "In provider implementations, when raising unsupported-operation errors, pass provider.GetProviderKey() to NewUnsupportedOperationError so error messages and ExtraFields.Provider reflect the provider's alias. Apply this pattern consistently across all provider files."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@core/providers/cloudflare/cloudflare.go` around lines 118 - 145, The
streaming call in ChatCompletionStream currently passes the hardcoded
schemas.Cloudflare to openai.HandleOpenAIChatCompletionStreaming; change that
argument to provider.GetProviderKey() so the provider alias is used consistently
(like in ChatCompletion, ListModels, and Embedding), updating the call in the
ChatCompletionStream function to replace schemas.Cloudflare with
provider.GetProviderKey() so logs/errors and ExtraFields.Provider reflect custom
aliases.
Adds blocked model support for virtual key provider configurations. Provider keys already supported both allowed and blocked models, but virtual key provider configs only supported allowed models. This PR adds the missing blocked model flow for virtual keys, so specific models can be denied at the VK provider-config level. * Added `blacklisted_models` to virtual key provider configs. * Added a configstore migration for the new `blacklisted_models` column. * Updated virtual key create/update handlers to validate, persist, and return blocked models. * Added governance checks to reject virtual key requests when the requested model is blocked. * Made blocked models take priority over allowed models. * Updated virtual key model filtering to respect blocked models. * Added `Blocked Models` UI under provider configurations in the virtual key create/edit sheet. * Added blocked model display in the virtual key details sheet. * Updated frontend governance types for virtual key provider configs. This follows the existing provider-key blocked model behavior instead of introducing a separate flow. Behavior: * Empty `blacklisted_models` means no models are blocked. * `["*"]` blocks all models for that VK provider config. * If the same model exists in both allowed and blocked models, the blocked list wins. * [ ] Bug fix * [x] Feature * [ ] Refactor * [ ] Documentation * [ ] Chore/CI * [x] Core (Go) * [x] Transports (HTTP) * [ ] Providers/Integrations * [x] Plugins * [x] UI (React) * [ ] Docs Local UI flow: Start Bifrost locally with embedded UI on port `9090`, then open: `http://localhost:9090/workspace/governance/virtual-keys` Steps tested: 1. Open Virtual Keys. 2. Create or edit a virtual key. 3. Expand a provider config. 4. Confirm `Blocked Models` appears below `Allowed Models`. 5. Select a blocked model and save. 6. Reopen the virtual key and confirm the blocked model is still shown. 7. Refresh the page and confirm the value persists. Runtime validation: 1. Configure a VK provider config with `allowed_models: ["*"]` and `blacklisted_models: ["<model-to-block>"]`. 2. Send a request through that virtual key using the blocked model. Expected: request is rejected. 3. Send a request through the same virtual key using a model not present in `blacklisted_models`. Expected: request succeeds. 4. Configure the same model in both `allowed_models` and `blacklisted_models`. Expected: request is rejected because blocked models take priority. 5. Configure `blacklisted_models: []`. Expected: existing virtual key behavior remains unchanged. Sanity checks: `go test ./framework/configstore/... ./plugins/governance/... ./transports/bifrost-http/handlers/...` UI check: `cd ui && pnpm build` Added a recording showing the new `Blocked Models` field in the virtual key provider configuration flow. https://github.com/user-attachments/assets/64efca01-0366-491c-b9e7-95dfa08eb0bc * [ ] Yes * [x] No BF-896 This change improves virtual-key governance by allowing specific models to be denied for a VK provider config. No provider secrets, customer keys, auth tokens, or PII are exposed or stored by this change. Existing provider-key behavior is unchanged. * [x] I read `docs/contributing/README.md` and followed the guidelines * [ ] I added/updated tests where appropriate * [ ] I updated documentation where needed * [x] I verified builds succeed (Go and UI) * [x] I verified the CI pipeline passes locally if applicable.
## Summary Improves E2E test stability for virtual key editing by handling a budget reset dialog that can appear after saving, and marks a known flaky bulk-rotate test as `fixme` until the underlying UI bug is resolved. ## Changes - Added `preserveBudgetUsageIfPrompted()` helper that detects the `vk-budget-reset-dialog` and clicks the preserve button if it appears after saving a virtual key. This prevents test failures caused by an unexpected dialog interrupting the save flow. - Marked `should bulk rotate selected virtual keys only` as `test.fixme` due to a UI bug where the checkbox selection state resets when the search input filters out a previously selected row. When `bulkRotateVirtualKeys` searches by name to select each key, the first key becomes deselected as the search narrows to the second, resulting in only the last key being rotated. ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Run the virtual keys E2E suite and confirm no failures occur on the save flow due to the budget reset dialog: ```sh cd tests/e2e npx playwright test features/virtual-keys/virtual-keys.spec.ts ``` The bulk rotate test will be skipped (`fixme`) and should not cause CI failures. ## Screenshots/Recordings N/A ## Breaking changes - [ ] Yes - [x] No ## Related issues N/A ## Security considerations None. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
## Summary Consolidates the Per User OAuth Postman coverage probes to align with the updated API surface, replacing the old register/authorize/token/consent/upstream endpoints with the new flow-based endpoints (`/api/oauth/per-user/flows/:flowId` and `/api/oauth/per-user/flows/:flowId/start`). ## Changes - Removed coverage probe requests for `Per User OAuth Register`, `Per User OAuth Authorize`, `Per User OAuth Token`, `Per User OAuth Upstream Authorize`, `Per User Consent VK`, `Per User Consent User ID`, `Per User Consent Skip`, and `Per User Consent Submit` - Added coverage probe requests for `Per User OAuth Flow Detail` (`GET /api/oauth/per-user/flows/coverage-probe-flow`) and `Per User OAuth Flow Start` (`GET /api/oauth/per-user/flows/coverage-probe-flow/start`) - Added `OAuth Callback (Coverage Probe)` to the raw text shapes validation map, replacing the three previously tracked OAuth probe entries ## Type of change - [ ] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [x] Chore/CI ## Affected areas - [ ] Core (Go) - [x] Transports (HTTP) - [ ] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test Run the updated Postman collection against a running Bifrost instance and verify that the new flow-based coverage probe requests return expected responses and that the response structure validation passes. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No security implications. These are test coverage probes only. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [x] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [x] I verified the CI pipeline passes locally if applicable
Closes maximhq#3411. Cloudflare Workers AI exposes an OpenAI-compatible surface for chat completions and embeddings under the per-account base URL https://api.cloudflare.com/client/v4/accounts/<account_id>/ai/v1 so the new provider sits firmly on the OpenAI-compat path, delegating chat / streaming / embeddings / list-models / responses to the shared openai handlers in the same pattern as Cerebras, Groq, etc. The one wrinkle is that there is no global default URL that omits the account id. NewCloudflareProvider therefore returns an error when network_config.base_url is empty rather than silently routing to a broken endpoint. What's wired up: - core/providers/cloudflare/cloudflare.go — Provider implementation, modelled on Cerebras. Supports chat (streaming + non-streaming), responses (chat-fallback), embeddings, list models. Everything else returns NewUnsupportedOperationError. - core/providers/cloudflare/cachedcontents.go — Mirrors the Cerebras unsupported-cached-content stubs so the Provider interface is satisfied. - core/providers/cloudflare/cloudflare_test.go — Comprehensive llmtests config gated on CLOUDFLARE_API_KEY + CLOUDFLARE_ACCOUNT_ID (skips when unset, mirroring the Cerebras test pattern), plus a unit test that locks in the "base_url is required" contract without needing network access. - core/schemas/bifrost.go — Cloudflare ModelProvider constant added to StandardProviders. - core/bifrost.go — createBaseProvider wires schemas.Cloudflare to cloudflare.NewCloudflareProvider. - docs/providers/supported-providers/cloudflare.mdx + docs/docs.json navigation entry. - docs/openapi/openapi.json + transports/config.schema.json — provider enums updated in all required spots. - ui/lib/constants/{config.ts,icons.tsx,logs.ts} — placeholder, key requirement, label, embedding-supported list, and a Cloudflare brand cloud icon. - .github/workflows/{pr-tests.yml,release-pipeline.yml} + scripts/test-docker-image.sh — CLOUDFLARE_API_KEY and CLOUDFLARE_ACCOUNT_ID added everywhere CEREBRAS_API_KEY is wired. Maintainers will need to set the matching repository secrets; without them the integration test skips cleanly. Verification: - go build ./... passes. - go test ./providers/cloudflare/... passes (TestCloudflare skips without keys; TestCloudflareRequiresBaseURL passes). - go test ./providers/... — all packages green except the pre-existing TestBifrostToGeminiToolConversion failure on main, which is unrelated to this change. - npm run lint (UI) — 0 errors, 387 pre-existing warnings unchanged. Doc reference: https://developers.cloudflare.com/workers-ai/configuration/open-ai-compatibility/
…verride, harden-runner allowlist Three review fixes from Greptile and CodeRabbit on maximhq#3604: 1. .github/workflows/scripts/test-docker-image.sh — the heredoc that writes config.json is single-quoted (correctly, since `env.XXX` strings are resolved by Bifrost itself), so $CLOUDFLARE_ACCOUNT_ID in the cloudflare base_url was being written literally and the integration test would hit an invalid URL when the secret is set. Substitute it after the heredoc with sed using a non-/ delimiter so the URL slashes don't need escaping. 2. core/providers/cloudflare/cloudflare.go — ChatCompletionStream now builds its URL with providerUtils.GetPathFromContext, matching ChatCompletion / Embedding / ListModels and respecting any context-set path override. 3. .github/workflows/release-pipeline.yml — added api.cloudflare.com:443 to all 4 step-security/harden-runner allowlists that already include api.cerebras.ai:443, so the Cloudflare integration tests can reach the upstream API under the egress-policy: block jobs.
Greptile flagged that the integration test would error with "unsupported provider: cloudflare" rather than passing once both CLOUDFLARE_API_KEY and CLOUDFLARE_ACCOUNT_ID are set in CI, because ComprehensiveTestAccount's three callbacks didn't know about the new provider. Adds schemas.Cloudflare to: - GetConfiguredProviders — pre-registers the provider on Bifrost startup, alphabetically next to Cerebras. - GetKeysForProvider — returns env.CLOUDFLARE_API_KEY with the same shape as the Cerebras key entry. - GetConfigForProvider — composes BaseURL from CLOUDFLARE_ACCOUNT_ID via fmt.Sprintf, since Workers AI's URL embeds the account id and there is no usable default.
…ovider Two more review fixes from Greptile and CodeRabbit on maximhq#3604: 1. Greptile (P1, confidence 3/5): every Cloudflare endpoint URL was constructed with a double `/v1/` segment because the documented base URL ended in `/ai/v1` and the provider also appended `/v1/...`, so live calls went to `…/ai/v1/v1/chat/completions` and would 404. The cause is that I diverged from the Cerebras/Groq convention — those providers have the base URL stop at the host (`https://api.cerebras.ai`) and append `/v1/...` per request. The fix is to stop the documented Cloudflare base URL at `/ai`, matching the cohort. Provider code is unchanged; only the documented / fixture URLs move. Updated: - core/internal/llmtests/account.go (test fixture) - .github/workflows/scripts/test-docker-image.sh (docker config) - core/providers/cloudflare/cloudflare.go (package doc + the constructor's error message that suggests the URL) - core/providers/cloudflare/cloudflare_test.go (URL the unit test asserts the constructor accepts) - docs/providers/supported-providers/cloudflare.mdx (user guidance + the caveat block that referenced the URL) 2. CodeRabbit (Major, outside-diff): SupportedBaseProviders omitted Cloudflare while StandardProviders and the custom_provider_config schema enums included it, so a schema-valid custom-provider config backed by Cloudflare would be rejected by the runtime allowlist. Added schemas.Cloudflare to SupportedBaseProviders alongside the other OpenAI-compat-friendly bases. Skipped two CodeRabbit suggestions intentionally: - Deep-copy NetworkConfig.ExtraHeaders to avoid a shared-map race. No provider in core/providers/* deep-copies it today (cerebras, groq, mistral, cohere, …); changing only cloudflare would make it the inconsistent one. If the race is real, it should be addressed repo-wide in a separate PR. - Replace `schemas.Cloudflare` with `provider.GetProviderKey()` in the streaming call. Same reason — every other OpenAI-compat provider hardcodes its own ModelProvider in HandleOpenAIChat CompletionStreaming (cerebras line 168, etc.). Matching the cohort for now.
5585b3e to
230cff5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/cloudflare/cloudflare.go`:
- Around line 47-71: The BaseURL is only whitespace-trimmed for the empty check
but later trimmed of trailing slashes on the original value, so leading/trailing
spaces can persist and break requests; fix by normalizing
config.NetworkConfig.BaseURL early (e.g. assign trimmed :=
strings.TrimSpace(config.NetworkConfig.BaseURL) and use that for both the empty
check and later assignment) and then apply strings.TrimRight on that trimmed
value before persisting; update uses around config.NetworkConfig.BaseURL,
strings.TrimSpace, and strings.TrimRight to ensure the stored BaseURL has no
surrounding whitespace and no trailing slash.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e3d05205-5e49-4947-8f92-78c05c97e220
📒 Files selected for processing (16)
.github/workflows/pr-tests.yml.github/workflows/release-pipeline.yml.github/workflows/scripts/test-docker-image.shcore/bifrost.gocore/internal/llmtests/account.gocore/providers/cloudflare/cachedcontents.gocore/providers/cloudflare/cloudflare.gocore/providers/cloudflare/cloudflare_test.gocore/schemas/bifrost.godocs/docs.jsondocs/openapi/openapi.jsondocs/providers/supported-providers/cloudflare.mdxtransports/config.schema.jsonui/lib/constants/config.tsui/lib/constants/icons.tsxui/lib/constants/logs.ts
✅ Files skipped from review due to trivial changes (3)
- docs/openapi/openapi.json
- docs/docs.json
- docs/providers/supported-providers/cloudflare.mdx
|
Pushed Fixes
Skipped, with reasoning
Verification: |
…rsisting CodeRabbit (Minor): the constructor checked strings.TrimSpace(config.NetworkConfig.BaseURL) for emptiness but then ran strings.TrimRight on the un-stripped original, so " https://api.cloudflare.com/.../ai/ " would pass the empty check and end up stored with the leading space intact, which would break request URL construction. Fix is one extra local: assign baseURL := strings.TrimSpace(...) up front, use it for both the empty check and the TrimRight before persisting. Adds an explicit unit-test case that constructs a whitespace-padded URL and asserts the provider builds cleanly, so this regression has a guard.
|
Pushed
|
8c3e42e to
b95e8e7
Compare
e389df7 to
a65fce4
Compare
|
@praveenkumarpranjal can you resolve the conflicts and pending comments on the PR |
fa15f50 to
ca190fc
Compare
Closes #3411.
Summary
Adds a Cloudflare Workers AI provider, hooked up across core, docs, UI, schemas, and CI.
Cloudflare exposes an OpenAI-compatible surface for
/v1/chat/completionsand/v1/embeddingsunder the per-account base URLhttps://api.cloudflare.com/client/v4/accounts/<account_id>/ai/v1, so this provider sits firmly on the OpenAI-compat path and delegates to the sharedopenai.HandleOpenAI*handlers in the same pattern as Cerebras, Groq, etc.The one wrinkle is that there is no global default URL that omits the account id.
NewCloudflareProvidertherefore returns an error whennetwork_config.base_urlis empty, rather than silently routing to a broken endpoint.What's wired up
Provider
core/providers/cloudflare/cloudflare.go— Provider implementation, modelled on Cerebras. Supports chat (streaming + non-streaming), responses (chat-fallback), embeddings, list models. Everything else returnsNewUnsupportedOperationError.core/providers/cloudflare/cachedcontents.go— Mirrors the Cerebras unsupported-cached-content stubs so theProviderinterface is satisfied.core/providers/cloudflare/cloudflare_test.go— Comprehensivellmtestsconfig gated onCLOUDFLARE_API_KEY+CLOUDFLARE_ACCOUNT_ID(skips when unset, mirroring the Cerebras test), plus a unit test that locks in the "base_url is required" contract without needing network access.Schema + wiring
core/schemas/bifrost.go—CloudflareModelProviderconstant added toStandardProviders.core/bifrost.go—createBaseProviderwiresschemas.Cloudflaretocloudflare.NewCloudflareProvider.Docs + config schema
docs/providers/supported-providers/cloudflare.mdxanddocs/docs.jsonnavigation entry.docs/openapi/openapi.jsonandtransports/config.schema.json— provider enums updated in all required spots (config provider map, fallback embedding provider enum, base provider type enum).UI
ui/lib/constants/config.ts—ModelPlaceholders.cloudflareandisKeyRequiredByProvider.cloudflare.ui/lib/constants/logs.ts—KnownProvidersNames,ProviderLabels, andEmbeddingSupportedProviders.ui/lib/constants/icons.tsx— Cloudflare cloud icon following the existing theme-aware pattern.CI
.github/workflows/pr-tests.yml,release-pipeline.yml, andscripts/test-docker-image.sh—CLOUDFLARE_API_KEYandCLOUDFLARE_ACCOUNT_IDadded everywhereCEREBRAS_API_KEYis wired (10 jobs total). Maintainers will need to set the matching repository secrets; without them the integration test skips cleanly.Verification
go build ./...passes.go test ./providers/cloudflare/...passes (TestCloudflareskips without keys;TestCloudflareRequiresBaseURLpasses).go test ./providers/...— all packages green except the pre-existingTestBifrostToGeminiToolConversionfailure onmain, which is unrelated to this change. I confirmed it fails onmainbefore any Cloudflare commits.npm run lint(UI) — 0 errors, 387 pre-existing warnings unchanged.Notes for maintainers
The Cloudflare brand icon I added is a clean cloud silhouette in the official brand orange (
#F38020). Happy to swap in the official Cloudflare wordmark from the press kit if you'd prefer — just let me know.Tool calling on Workers AI is model-dependent; the test config keeps
ToolCalls: falsefor the first cut. We can flip it on later for catalog entries that advertisefunction_calling: true.